test: a ledger of which checks have ever been red (#918) - #925
Conversation
Check results were prose. `check`, `check_num` and `check_text` printed PASS or FAIL and nothing else, so proving that a mutation reddened one NAMED check meant grepping text -- which is how every mutation proof in this repository is currently made, a person reading `FAIL <name>` out of a log and retyping it. That is also how a reverted guard once reported plain green while the check count fell from 190 to 186: the suite passed, and the only evidence anything had changed was a number nobody was comparing. THE FIX IS NOT A SECOND EMITTER. A second source of truth for how many checks ran is the defect this family of issues exists to close. lib.sh had ELEVEN places that bumped PGC_CHECKS, each with its own outcome line beside it -- eleven chances to add a twelfth and forget the line, which is exactly what projections.sh's expect_fail did with ten call sites for as long as it existed. So counting a check and recording it are ONE operation, pgc_record. A helper cannot report an outcome without being counted, and cannot be counted without reporting one, because no code path does either alone. `checks run: N` and the N record lines are the same increment seen twice. Eleven sites became one, and the arm that holds it is structural: lib.sh may bump PGC_CHECKS in exactly one place, and that place must be pgc_record. The record is tab separated -- suite, name, verdict, reason -- so a check name containing spaces survives. The reason carries #915's REASON_CODE, which is what makes this more than a reformat: an unrunnable check is distinguishable from a passing one without parsing prose. A verdict pgc_record does not recognise is recorded as a FAIL rather than dropped, because dropping it would leave PGC_CHECKS bumped with no outcome recorded -- the reconciliation pgc_summary already refuses. THE HUMAN LINES DID NOT MOVE. DISPLAY is passed to pgc_record whole rather than composed inside it, and both harnesses pin the exact strings for check, check_text, check_num and check_unrunnable. 3,762 call sites, with suites, selftests and CI all grepping `^PASS` and `^FAIL`, is far past what a careful refactor can be trusted on. PGC_SUITE is resolved once at load rather than per check: pgc_record runs at every one of those call sites, and a basename fork at each is 3,762 forks a suite does not need. The runner reconciles the two artifacts per suite: a log states `checks run: N` and carries N records. That cannot fail by drifting, since one function does both, but it can fail -- a suite killed mid-way, a truncated log, a helper that prints an outcome without recording it. A log with no count at all never reached its summary, which is a different fault from a miscount and is reported as one rather than reading as a clean reconciliation. Stacked on #916, which supplies pgc_log_shows_accounting: only a suite that reached its summary has a count to reconcile against. Evidence: selftest exit 0, 535 checks, 535 records, 0 failures; 14 pytest tests; shellcheck -S error rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…adable-results # Conflicts: # test/run_all_versions.sh
… made the reconciler wrong (#917) Both from OffgridwithJD's review, and the first is embarrassing in the specific way that makes it worth writing down. FOUR FORKS PER RECORD, IN THE FUNCTION THAT HOISTS PGC_SUITE OUT TO AVOID THEM. pgc_record ran `$(printf ... | tr)` twice to blank tabs -- two subshells and two tr processes -- at every one of 3,762 check sites, three lines below a comment explaining that a basename fork per check is 3,762 forks a suite does not need. Parameter expansion does it free. Measured on an idle box, 2,000 calls, output identical on every input including a real tab, a leading tab and a trailing one: printf | tr in $( ) 3.1577 ms per call ${var//tab/ } 0.0096 ms per call ratio 331x across 3,762 checks 11.9 s of pure fork overhead against 36 ms DIRECT COUNTER WRITES MADE THE RECONCILER WRONG, and the two rules disagreed. Selftest 320 blessed a direct PGC_CHECKS bump that records an outcome nearby; pgc_reconcile_records requires a RESULT line. Thirteen sites across ten suites took the first path, so on a failure the reconciler added `records=N but the log states checks run: N+1` on top of the real failure. The fix is not to soften the reconciler. Counting a check and recording it are one operation, which is this change's whole argument, and a direct write is a check counted with nothing recorded -- the hole that argument cannot have. All thirteen now call pgc_fail, which lib.sh's own header already calls "the only supported way to add a check from outside this file". Selftest 320 gains the stronger rule that was not satisfiable until now: no suite using lib.sh's accounting writes PGC_CHECKS directly. bench_guards keeps its own counter under the same name and never sources lib.sh, and is exempt by measurement rather than by name. TWO OF MY OWN ARMS WERE WRONG ON THE WAY THROUGH, both caught by running them. The old premise required FIVE direct writes to EXIST, which is a premise about the corpus rather than about the sweep -- so converting the thirteen turned it red for exactly the reason the change is for. A premise that fails when the thing it guards is fixed is the wrong premise. It now asserts the sweep read something, and both rules are proven on fixtures: the original flags a bump with no outcome and allows one with an outcome, the stronger one flags that same allowed bump, and both exempt a private counter. Two arms reading "[]" over a clean corpus are satisfied by a sweep that classifies nothing. And those fixtures, written out literally, made this file flag its own three generator lines -- the same mistake selftest 080's control avoids by living in a quoted heredoc. The bump is now assembled from the variable name. Evidence: selftest exit 0, 581 checks, 581 records, 0 failures; 133 pytest passed (the 35 errors are /usr/local/pg18a absent on this host, identical on main); shellcheck rc=0 across test/, selftest/ and bench/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Closes #918, phase 4 of #858, and the honest version of it. Nothing recorded whether a check had ever been red. That is the gap that let 39 checks across 35 suites ship unable to fail, three of them inside the suite whose whole purpose is to stop exactly that. The gate answered "did anything print FAIL" and had never answered "could anything print FAIL". WHAT THIS LEDGER CLAIMS, AND WHAT IT DOES NOT. It records that a named check WAS OBSERVED RED in a recorded run. It does NOT claim the check is proven able to fail: that is a stronger statement, it needs a named mutation applied deliberately, and conflating the two would put a claim in the ledger that nothing measured -- the `defeated: 0` shape from VACUITY_MODES section 1, a number that reads as evidence and is not. SO EVERY ENTRY CURRENTLY READS `never`, and that is the finding rather than an embarrassment. #918 asks "nothing records whether a check has ever been red"; the answer this ships is "and now something records that almost nothing has". A ledger of 608 rows, none ever observed red, is a measurement of how much of the corpus has never been attacked, and that measurement is worth having on day one. WHAT FILLS IT. Not only deliberate mutation runs. Every real CI red fills it, every flake, every bisect, and those arrive whether anyone remembers or not. A mutation run is the deliberate accelerator, not the only source. Raised by OffgridwithJD, and it matters because "only a mutation run can retire debt" invites someone to build a mutation gate before it is needed. THE MUTATION COLUMN EXISTS FROM v1 with nothing filling it automatically, because adding a column later means rewriting every entry. If an entry can record WHICH mutation reddened a check, the catalogue a mutation gate would need builds itself out of work people already do by hand -- the vacuity branches are writing nine to eleven per change, each chosen to revert one property. A RENAME IS REPORTED, NOT SILENTLY ABSORBED. The ledger is keyed by check name, and names here are prose that gets rewritten freely -- which is most of why #917 exists. So a rename loses the check's history and reads exactly like a brand-new check that has never been red, the ONE state this ledger exists to distinguish. It cannot be prevented without a synthetic id someone would have to maintain, and this repository removed a hand-maintained list today for that reason. So a name that appeared while another disappeared is NAMED. Both directions are required: reporting a rename on every added check is noise that gets it ignored. A DUPLICATED NAME SHARES ONE ROW, so one of the two going red would mark BOTH as observed red -- a claim about a check nothing attacked. Also reported rather than prevented. The real corpus carries four today, which is how it was noticed at all: 612 records reduce to 608 rows. TWO TRACKED FILES CARRY THE DEBT, per #858's own constraint. check_ledger.tsv and check_ledger_budget.txt are in the tree, so a change to either is a diff a reviewer sees. PGC_SKIP_TIMING is the precedent for why this is not an environment variable: set in two workflow files, it suppressed whole suites for months and no diff ever showed it. Both numbers may only go down. The second number is the one that is easy to forget: suites_not_covered is 250 of 251, because the ledger can only be seeded from suites whose logs exist, and the matrix does not preserve them. Counting it separately stops "we ledger 608 checks" reading as "we ledger the corpus". It is a real limit, stated rather than hidden, and it burns down as suites are seeded. The gate refuses a check the ledger has never seen, so a new check cannot enter as silent debt -- while the existing 608 are grandfathered, because a gate that fails on 3,762 unledgered sites is one somebody disables under deadline. Evidence: selftest exit 0, 612 checks, 0 failures; the gate rc=0 against the committed files; 27 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…#917) Reported by OffgridwithJD, and the finding is that the four duplicate check names I reported from #918 are not four instances -- they are a CONVENTION, and the count grows with every part anyone writes. The phrasing is the cause. `premise: the pytest layer is where THIS PART thinks it is` says "this part" precisely so the sentence can be copied into any part, and main already carries two copies of it and two of `premise: the harness library is where this part thinks it is`. All six of the reviewer's own in-flight branches were adding more. SO (suite, name) IS NOT A KEY OF CHECKS. It is a key of check NAMES, and the two differ by however many parts share a boilerplate premise. One of them going red would mark every sharer as observed red -- a claim about a check nothing attacked, which is exactly what #918's ledger must not make. pgc_record now derives the part from BASH_SOURCE: the first frame that is not lib.sh. Not a convention change, so the next part written the same way is keyed correctly without anyone remembering, and a premise moving between parts stops being indistinguishable from a rename. Parameter expansion only -- no basename fork -- because this runs at every one of 3,762 call sites, which is the mistake this same function already made once. RESULT <TAB> suite <TAB> part <TAB> name <TAB> verdict <TAB> reason MEASURED OVER A REAL RUN, 583 records: distinct (suite, name) 579 -> 4 collisions distinct (suite, part, name) 582 -> 1 38 distinct parts are named. The one survivor is a GENUINE duplicate -- 340-the-binary-must-be-built-from asks `premise: the fixture fingerprints at all` twice within the same part -- which is a real defect the ledger can now name precisely instead of losing it among convention artifacts. Evidence: selftest exit 0, 583 checks, 0 failures; pytest; shellcheck rc=0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…onvention (#918) Reported by OffgridwithJD, who found it by asking whether their own six branches added duplicate check names. All six did. THE FOUR DUPLICATES I REPORTED ARE NOT FOUR INSTANCES, THEY ARE A CONVENTION. `premise: the pytest layer is where THIS PART thinks it is` says "this part" precisely so the sentence can be copied into any part, and main already carries two copies of it and two of `premise: the harness library is where this part thinks it is`. The count is the count TODAY and grows with every part anyone adds. So a ledger keyed on (suite, name) is not a ledger of checks. It is a ledger of check NAMES, and the two differ by however many parts share a boilerplate premise -- with the consequence this ledger cannot have: one sharer going red marks every other as observed red, a claim about a check nothing attacked. THE KEY IS NOW (suite, part, name). The part comes from pgc_record, derived from BASH_SOURCE, which #917 adds in the commit below this one. Not a convention change, so the next part written the same way is keyed correctly without anyone remembering, and it closes a blind spot in the rename detector: a premise moving between parts was indistinguishable from a rename and is now an appearance and a disappearance in two different parts, which the detector does not pair. Measured over a real run, 583 records: distinct (suite, name) 579 distinct (suite, part, name) 582 One duplicate survives, and it is a GENUINE one rather than a convention artifact: 340-the-binary-must-be-built-from asks `premise: the fixture fingerprints at all` twice within the same part. Naming that precisely, instead of losing it among three copied premises, is the point. The reviewer also renamed the premises in all six of their own in-flight branches so each names its own subject -- thirteen renames, each part now contributing zero duplicated names -- which is why this number does not grow by six the moment those land. They left main's copies alone as this PR's scope, which is right: this change reports rather than prevents. Re-seeded from a green run, keyed the new way: 613 rows, none ever observed red. Evidence: selftest exit 0, 614 checks, 0 failures; gate rc=0 against the committed files; 140 pytest passed; shellcheck rc=0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…tor caught the rename (#918) OffgridwithJD looked at the collision that survived the (suite, part, name) key and found it is not a repeat of one question. At 340:268 and 340:894, `premise: the fixture fingerprints at all` is asked once of the SOURCE-PARTITION fixture and once of the LOCALE SWEEP: 268 _fs_before="$(pgc_source_fingerprint "$_fs_rp")" -> did the a.c/b.c partition fixture produce a fingerprint at all 894 _lc_vals="$_lc_vals $(LC_ALL=... pgc_source_fingerprint "$_lc/tree")" -> did every locale in the sweep produce one Two questions about two different code paths, sharing a sentence. That is the case where a shared ledger row is WRONG rather than merely imprecise: one failing marks the other's premise as observed red, and they are not about the same code. I FIXED IT RATHER THAN REPORTING IT, which departs from this PR's stance, and the distinction is worth stating. "Report rather than prevent" is right about the CONVENTION -- `premise: the pytest layer is where THIS PART thinks it is` is phrased to be copied, rewriting the corpus's shared premises is churn, and the tool exists to surface them. This is not that. It is one file, two lines, and the two premises are about different subjects. Naming them apart takes the corpus to ZERO collisions: 614 records, 614 distinct keys. AND IT IS THE RENAME DETECTOR'S FIRST TEST ON REAL DATA rather than fixtures. Run against the ledger seeded before the rename: possible rename: premise: the fixture fingerprints at all -> premise: every locale produced a fingerprint (in harness_selftest/340-the-binary-must-be-built-from, history: last red never) rename scan: appeared=2, vanished=1 appeared=2 against vanished=1 is honest: the two premises shared ONE row before, so one of the new names has no predecessor to pair with. Re-seeded: 614 rows, none ever observed red. Evidence: selftest exit 0, 614 checks, 0 failures, 0 collisions; gate rc=0 against the committed files; shellcheck rc=0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
|
Reviewed The finding:
|
linuxhikerpm
left a comment
There was a problem hiding this comment.
Blocking at exact head 671a946059ddaf88b3289d28ee891c20ab8fdb0d: the ledger is not yet an operational or fail-closed gate.
-
pgc_ledger.pyis never invoked by the runner or CI. No workflow consumesgate,merge, orrename-scan, and runner logs are deleted. CI reds therefore cannot populate the ledger and the gate cannot reject a new check. -
Missing/malformed inputs false-green.
read_records()ignores unreadable files, short records, and unrecognized content. Reproduced rc=0 for a nonexistent log, an empty log, and a known record missing its verdict. Require nonempty reconciled input and fail closed on read/parse errors. -
Mutation attribution is invocation-wide. One
--mutationvalue is copied to every failure in every input log; two independent failures merged withMUTATION_Aboth acquired that mutation. The issue requires per-check attribution derived from the run. -
Multi-log handling is incorrect. Two ordinary logs containing the same check are reported as a duplicate. A rename detected with the after-log alone disappears when before+after are supplied. Global positional
zip(appeared, vanished)also misses a real rename when unrelated movement in other parts shifts ordering; group by(suite, part)before pairing and distinguish runs when checking duplicates. -
The tracked budgets are not monotonic. Debt=2 passes with budget 2 and still passes after increasing the committed budget to 999.
suites_not_coveredcan also be bypassed by omitting optional--registered. Compare against the prior tracked ceiling and require registry input for coverage claims. -
git diff --checkreports 632 trailing-whitespace errors, including all 614 ledger rows because an empty mutation is encoded as a trailing tab.
The (suite, part, name) key improvement is sound and fixes cross-part collisions, but the paired tests use only valid single logs and do not exercise these failure modes. Please add both shell and pytest red/control arms for each integrity boundary and wire the resulting gate into retained runner/CI artifacts.
|
Two additional exact-head findings from the full adversarial pass:
These supplement the changes-requested review at the same head; the existing six blockers still stand. |
|
Adversarial pass on 1. The mutation column silently overwrites, so it cannot accumulate a catalogueThat column exists so the mutation catalogue builds itself out of work people already do by hand. It keeps only the last one: No warning, no append, no record that M1 ever existed. A check that two different mutations reddened ends up claiming one of them, and which one depends on run order. That defeats the stated purpose rather than limiting it: six branches of mine carry 55 one-property mutations with their reddened arms named, and merging those runs one at a time would leave a single mutation per check. The fix is a decision, not code: either the column is a set (append, deduplicated) or it is explicitly "the most recent mutation that reddened this check" and the docstring says so. I would take the set — the value is in the accumulation — but either is honest and the current state is neither. 2. Drift is one-directional: a ledger row whose check no longer exists is kept silentlyThe gate reports
3. A stale number inside the file whose whole subject is stale numbers
Trivial to fix and worth fixing precisely because of where it is. On the budget file being typed at allI went looking for this as a defect and came away agreeing with it, so I will say so. Verified and soundMerging one run twice is byte-identical. A later date advances last-red; a subsequent green run does not erase it. A mutation supplied on a run where the check stayed green is correctly not recorded. The gate refuses a record with no ledger row by name, with a control at rc=0. 614 records give 614 distinct keys after the 340 rename, and the key's part comes from One merge note, recorded here rather than only in messages: |
…#917) Blocking review by @linuxhikerpm at the exact head, and the finding is a hole in this change's own argument. check_timing and check_ratio_needs_quiet_machine, under PGC_SKIP_TIMING=1, printed a human SKIP line and returned. Driven before the fix: SKIP a timing check (PGC_SKIP_TIMING: wall-clock measurement) SKIP a ratio check (PGC_SKIP_TIMING: wall-clock ratio) -> PGC_CHECKS=0 PGC_PASSED=0 PGC_FAILED=0 PGC_UNRUN=0 Two outcomes a reader sees, invisible to the count and to the records both, in the change whose whole argument is that those are one operation. And nothing reached those branches: no arm in any harness mentioned either helper, so removing both emitters left everything green -- which is how they found it. SKIP IS A FOURTH OUTCOME, counted like the other three. `checks run:` now reports the checks a suite ENCOUNTERED rather than the ones it managed to evaluate, and pgc_summary reconciles four counters against that count instead of three -- the same shape one term wider, preserving the property its own comment argues for. IT IS DELIBERATELY NOT check_unrunnable. That third state exists for a check whose INPUT was absent and it exits the suite INCOMPLETE. CI sets PGC_SKIP_TIMING on every run, so routing these through it would turn every run red. A wall-clock check deliberately not asked on a shared runner is a different thing from one that could not be answered, and the ledger should be able to tell them apart. THE ALL-SKIPPED SUITE WOULD HAVE REPORTED PASSED. Before the fourth counter a skipped check left PGC_CHECKS at zero, so `if PGC_CHECKS = 0` caught that case by accident; counting it would have made such a suite report PASSED with nothing behind it. The condition now says what it always meant: PASSED + FAILED + UNRUN. REMOVAL PROOF, which is the thing their finding asked for. With both emitters deleted -- the mutation they applied, which used to leave everything green -- NINE named arms redden across both harnesses, and lib.sh restored byte-identical (md5 684272397ed5bb8a21a8b819a3066c59 before and after). THE DOCUMENTATION MISMATCH IS REAL AND NARROWER THAN REPORTED. The record has FIVE columns after the RESULT marker -- suite, part, name, verdict, reason -- and lib.sh, selftest 400 and TESTS.md all still said four, missing `part`. Fixed in all three. There is no `mutation` column in a record; that belongs to the LEDGER (#918), which keys on (suite, part, name) and records which mutation reddened a check. A record is one observation, not a history, and the docs now say so. The accounting line changing shape means both readers on main move with it -- the two regexes in run_all_versions.sh -- along with eleven fixtures across selftest 320, selftest 390 and test_suite_accounting.py. That coupling is exactly what #916's producer-versus-reader arm exists to catch, and it would have caught it. Evidence: selftest exit 0, 598 checks, 598 records, 0 failures; 23 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…is swept (#917) Both from OffgridwithJD, re-swept against this head rather than carried over. THE RECONCILER CAUGHT THE DEFECT AND DESCRIBED THE BOOKKEEPING. A check inside a piped loop runs in a SUBSHELL, so its counter bump dies there while its outcome and its record both reach the parent's stdout. Driven: check "direct one"; check "direct two" printf 'three\nfour\n' | while IFS= read -r n; do check "piped $n" a a; done -> four PASS lines, four RESULT lines, PGC_CHECKS=2 pgc_reconcile_records reported `records=3 but the log states checks run: 1`, which is true and useless: a reader who has not met this has no route from two numbers to a pipeline. It now names the cause, and the two directions get different causes -- more records than counted is a lost subshell, fewer is a counter bumped without going through pgc_record. AND THE SHAPE IS SWEPT, the way selftest 080 sweeps its cousin. Latent today: four piped loops in the tree, none with a check inside, so the sweep reports zero and four fixtures prove it can fire -- a rule whose only evidence is that the corpus happens to be clean is not a rule. THE SWEEP WAS WRONG TWICE BEFORE IT WAS RIGHT, both found by running it over the real corpus instead of reading it. Requiring the closing `done` to be alone on its line left the scanner inside a loop for the rest of any file whose loop ended `done)"`, flagging every later check: 27 hits against a true zero. Then a loop written entirely on ONE line inside a command substitution opened a block that never closed -- twelve of those false hits were in the sweep's own file. So it opens only on a line that opens a loop and does not close it, and closes on a `done` token wherever it sits. Four fixtures now pin that: a check inside a piped loop is found, one in a process-substitution loop is not, one after a one-line loop is not, and a piped loop with no check in it is not. The diagnostic message is ASSEMBLED rather than written out, because spelling the shape made the sweep flag the line that warns about it -- selftest 080's control problem, third time tonight. ONE NOTE, NOT A DEFECT. pgc_log_shows_any_accounting's `checks run:` alternative answers yes to an accounting line of any shape, so it MASKS a change to that line: a producer moving without its readers would leave the population reconciliation green while the accounting one reddened. Two checks disagreeing about one log is a worse signal than either failing. It cannot happen inside one tree, so it is recorded where someone debugging a half-merge would look. Evidence: selftest exit 0, 608 checks, 0 failures; 23 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
jd's standing rule is that no PR ships without its CHANGELOG and docs in the same PR. `git diff --name-only main HEAD -- CHANGELOG.md` was 0 files for this branch, which OffgridwithJD caught: between this and #918 the stack adds pgc_record, a machine-readable RESULT format across every call site, a fourth counted outcome, three selftest parts and two tracked data files, and none of it was recorded. The entry covers what a reader of the release notes needs: why one counter rather than eleven, why the record names the part and not just the suite, why a skipped wall-clock check is counted, and what the matrix now reconciles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…annot deadlock (#918) jd's direction: shipping the recording half alone would be shipping something that does nothing. A ledger nothing feeds and nothing reads is not a staged feature, it is a data file with no producer and no consumer -- the `defeated: 0` shape one level up. So the gate is fixed rather than dropped. Ten findings, from @linuxhikerpm and OffgridwithJD, every one reproduced first. IT IS NOW FED AND IT NOW REFUSES. Nothing in the repository called the tool: zero references in .github/, zero in the runner, so "the gate refuses a check the ledger has never seen" was false as written. run_all_versions.sh now runs it before it removes the build directory, which is the only place a matrix run can reach every suite's log. CI verifies; humans commit the ledger, because a ledger CI rewrote by itself would be a file nobody reads changing under everybody. THE DEADLOCK WAS THE DESIGN, NOT THE NUMBER. Bounding `checks_never_observed_red` means every added check breaks the gate: a new check enters as `never`, so the only way to land one was to raise a number the file says in capitals may only fall. It shipped at 614 rows, 614 never, ceiling 614. The two numbers are different kinds of thing and the file now says so. `checks_never_observed_red` is a CENSUS, asserted to match the ledger in both harnesses so it cannot drift. `suites_not_covered` IS a ceiling, because adding a check to a covered suite does not move it, and the gate compares it against the previously committed value and refuses an increase -- so "may only fall" is mechanism rather than prose. AND THE REFUSAL IS RESTRICTED TO SUITES THE LEDGER COVERS, which is the MEANING of that ceiling rather than a softening of the gate. Without it the gate refuses every check of all 250 uncovered suites and reddens the whole matrix on its first run, which is a gate somebody turns off within the week. It tightens on its own as suites are seeded, and an arm pins that a new check in a NOW-covered suite is refused again. FAIL CLOSED. A nonexistent log, an empty one and a record missing its verdict each returned rc=0. An integrity failure that reads as a clean run is worse than no gate because it certifies. They return 2, distinguishable from a real refusal at 1, and --registered is required rather than silently skipped. THE MUTATION COLUMN ACCUMULATES a set rather than overwriting, because keeping only the last one records the most recent attack rather than the catalogue the column exists to become. One --mutation cannot be attributed across several logs at once. MULTI-LOG HANDLING was wrong in two directions. The same check in two logs is two RUNS and was reported as a duplicate; the same name twice in ONE log is the duplicate, and is what is reported now. Renames are grouped by (suite, part) before pairing, because a global positional zip misses a real rename whenever unrelated movement elsewhere shifts the ordering -- and a before-log and an after-log together are refused rather than silently finding nothing, since the vanished name is present in the union. 614 rows ended in a tab, because an empty mutation was an empty last field. An absent mutation is now `-`; `git diff --check` reports nothing. Evidence: selftest exit 0, 638 checks, 0 failures; 9 pytest; shellcheck rc=0; docs_style PASSED; git diff --check clean. Both harnesses carry red-and-control arms for every integrity boundary above, including the deadlock as its own arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…s' into feat/918-mutation-ledger # Conflicts: # CHANGELOG.md
|
Treated as new, as you asked. I ran the thing the redesign turns on — a real PG 17 matrix on The blocker: CI is not running, and will not start
The cause is the base. This PR targets One file, one region: your entry and #923's under the same heading. It is fed, and I can prove it from a real runFull So the gate reached the real logs of a real matrix, and And the refusal works. Driven with a record the ledger has never seen, in a covered suite: Fail-closed holds where you said it does — a nonexistent log, an empty log, a malformed record, and a missing The finding: "the ceiling may only fall" is true of the tool and false of the wiring
The monotone block is guarded by
So the tool is right, and 410's scratch-repo arms prove it. What nothing does is compare the real committed budget against git. And adding the flag at that call site does not fix it. The arm that closes it belongs where the repository is rather than where the build copy is: compare the working budget against What I checked and am not relitigatingThe runner does invoke the gate, it runs before On shape, since you askedThe census/ceiling split is right, and the reason is stated correctly: every new check enters as One consequence worth naming out loud: today the ledger covers one suite, so the gate refuses unknown checks in A main-tree defect this run surfaced, which is not yours to fix hereThe same report prints two different "accounted" totals: 237 and 235, differing by exactly 2. The population line uses the wide reader ( |
…ng (#918) Reported by OffgridwithJD, measured on the shipped form. I wrote that the gate "now refuses to see the ceiling raised above its previously committed value". That was true of pgc_ledger.py and false of run_all_versions.sh: the runner's exact invocation, no --against rc=0 the raise is not refused --against HEAD, absolute path rc=0 "no prior ceiling to compare" --against HEAD, repo-relative path rc=1 correctly refused THE MIDDLE LINE IS THE ONE THAT MATTERS. `git show REF:PATH` needs a repo-relative path and the runner passes an absolute one inside a copied build directory, so _committed_budget returned None and the gate printed a note that READS LIKE A PASS while the ceiling it was asked to enforce went unchecked. Asked to compare, unable to compare, is not the same as nothing to compare -- and that is the fail-open shape this whole change is about, in the code that closes it. So the tool resolves the path itself, through the budget's own git toplevel, and every failure to resolve it is an ERROR. The caller no longer has to know. WHICH REF IS NOW A DECISION RATHER THAN A DEFAULT. `--against HEAD` compares a committed file against ITSELF: for any change already committed the working budget and HEAD's are identical, so it catches only an uncommitted raise. The property that matters is that a branch may not raise the ceiling relative to MAIN. The runner prefers origin/main, falls back to HEAD, and PRINTS the fallback and what it costs, because a silent fallback is a gate quietly enforcing less than it claims. THE SCRATCH-REPO ARMS WERE NECESSARY AND NOT SUFFICIENT, which is the gate-nothing-invokes finding one level down: they proved the tool while no wired invocation exercised it. There are now arms in the REAL tree at the REAL path -- an absolute path resolves rather than shrugs, a budget git has never seen is an integrity failure rather than a note, and raising the tracked ceiling in place is refused, with the file restored byte-exact. Also in this commit: the merge of #923's base, whose CHANGELOG entry conflicted with this one. Both entries are kept, #917 then #918, since they describe two changes under one heading. #925 was CONFLICTING against its base, which is why no CI had run on it. Evidence: selftest exit 0, 678 checks, 0 failures; 145 pytest passed (the 35 errors are /usr/local/pg18a absent on this host, identical on main); shellcheck rc=0; docs_style PASSED; git diff --check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Reported by OffgridwithJD, and it is my own sentence one level up. `origin` is not a fixed thing. In a contributor's clone it is their FORK -- they measured theirs 446 commits behind upstream -- so `--against origin/main` compared the ceiling against a 16-day-old main and printed that it had compared. THE DIRECTION IS WHAT MAKES IT FAIL OPEN. The ceiling may only fall, so an older main carries one that is higher or equal, and a raise passes whenever the stale prior is high enough. Never falsely red, silently weaker, with a line that reads like the enforcement happened. "Asked to compare, unable to compare, printing a note that reads like a pass" was the previous finding; this is the same shape with "compared against the wrong thing" in place of "could not compare". AND THE FALLBACK WAS THE SAME SHAPE AGAIN. When origin/main did not resolve the runner printed the cost and proceeded with `--against HEAD`, which compares a committed file against itself and therefore catches nothing for any change under review. A fallback that enforces less while saying so is still a gate enforcing less, and one level down I had already made an unresolvable prior an error. SO THE PRIOR IS RESOLVED, AND NEVER GUESSED. `--against auto` takes GITHUB_BASE_REF, which in CI names the PR's target and IS the prior by definition, or the local main's configured upstream outside CI, which is the per-clone answer to "which main is mine". Neither available is rc=2. The ref used is printed, so a reader can see which prior the comparison actually made. AND CI MUST FETCH THAT BASE. actions/checkout takes one ref at depth 1 and the suites job set no fetch-depth, so the base branch is absent and `auto` would stop the run -- correctly, but for a reason the workflow owns rather than the author. The suites job now fetches it at depth 1, guarded on github.base_ref so a push build does not fail on it. Only the file at that commit is read. Arms for all of it, including the two that would have caught me: no base ref and no upstream is an integrity failure with its reason named, and a GITHUB_BASE_REF whose ref is absent says the checkout needs to fetch it rather than falling back. Three arms testing the design this replaces were deleted rather than left to pass against nothing. Evidence: selftest exit 0, 686 checks, 0 failures; 9 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
|
The CI half of the
So in CI Fixed at One residual on the non-CI path, measured in the clone the new docstring cites:
I am not arguing for a different ref — "the configured upstream of the local main" is the right answer to "which main is mine" and I cannot see a better one that does not guess. I would add the distance: git rev-list --count <prior>..HEADprinted beside the ref. "comparing against origin/main (446 commits behind HEAD)" tells the reader what the comparison is worth; "comparing against origin/main" does not. Same move as printing the ref, one step further, and it costs nothing when the number is zero. Everything else in my earlier review stands as written, including the matrix evidence on |
…anged and the destination did not (#918) Reported by OffgridwithJD, who measured it in their own clone rather than predicting it. `main@{upstream}` is the per-clone answer to "which main is mine", and in a contributor's setup it resolves to their FORK -- `git push -u origin main` is what sets that config. Theirs is 446 commits behind upstream, so `--against auto` outside CI lands on exactly the ref the hardcoded `origin/main` did. The route changed; the destination did not. It is not the same defect: the ref used is printed, so a reader can see `origin/main`. But a reader cannot see that it is 446 commits stale, and the direction still fails open -- the ceiling may only fall, so an older main carries a higher one and a raise passes whenever the stale prior is high enough. There is no better ref to pick that does not guess, and I am not going to guess. So the weakness is made VISIBLE instead: ceiling against origin/main (446 commits behind HEAD): 250 -> 250 Naming the ref told a reader WHICH prior was used. This tells them what the comparison is worth. It costs nothing when the number is zero, where the label is omitted entirely. AND THE CI HALF IS NOW MEASURED RATHER THAN CONSTRUCTED, from my own run 34502198282 on fb87237: origin/main does not resolve here, so the ledger ceiling is ceiling against HEAD: 250 -> 250, which does not rise The fallback fired, the prior was HEAD, and the comparison was a committed file against itself -- catching nothing for any change under review, in the only place this gate runs for real, with a line above it that reads like enforcement. The previous commit removed that fallback and made CI fetch the base; this is the evidence it needed to be removed rather than kept with a warning. Evidence: selftest exit 0, 690 checks, 0 failures; 9 pytest; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
|
CI is red on From the I reached the same failure independently before seeing this. A full matrix run on So Three states, not twoThe distinction needed here is the one this branch already drew one level down:
The middle row is the first-landing case, and it is genuinely nothing to compare rather than could not compare. It is also self-clearing: once the file is on A second, smaller one in the same four linesThe runner prints for an integrity failure. The gate takes care to distinguish The half that is verifiedBoth of those runs reached the gate with real logs from a real matrix, printed the census and the coverage, and reported |
…wo failure kinds are different (#918) THE GATE CAUGHT ITS OWN BOOTSTRAP the first time it ran in CI, which is the right outcome for a rule that could not be satisfied. From run 34503924812: ledger integrity failure: --against refs/remotes/origin/feat/917-machine-readable-results was given, but test/check_ledger_budget.txt does not exist at that ref #925's base is #923's branch, where the budget does not exist because THIS change adds it. So `auto` resolved the base correctly, fetched it, found no prior, failed closed, and reddened the matrix -- and a PR introducing the file could never pass its own gate. OffgridwithJD hit the identical failure independently in their own clone, resolved to their fork, where the file is also absent. THREE STATES, NOT TWO, which is the distinction this change already draws one level down: the prior ref does not resolve ERROR. Asked to compare, unable to. the ref resolves, file absent there NO PRIOR. Nothing could have been raised relative to a file that did not exist. the ref resolves, file present COMPARE. The middle one is the first-landing case and it is genuinely "nothing to compare". It is expressed as the PROPERTY -- absent at the prior -- rather than as a flag or a date, so it clears itself: once the file is on main every future base carries it, and there is no exemption left for anyone to forget to remove. It is not a hole. Deleting the budget on a branch and re-adding it higher does not reach it, because the file still exists at the prior and the comparison happens. AND THE RUNNER COLLAPSED THE TWO FAILURE KINDS. The gate distinguishes rc=1, a real refusal whose fix is to regenerate the ledger, from rc=2, the gate unable to do its job at all. The runner reported both as "has a check the ledger has never seen" -- sending the reader at a repair that cannot help, three lines below the gate's own "new this run=0", which says the opposite. Also OffgridwithJD, from the CI log of this branch. It now branches, and both arms still fail the major. THE ARMS FOR IT WERE WRONG TWICE, both times in the same way. They counted occurrences inside `grep -A6`, `-A8` and `-A12` windows, and every one broke the moment the call site gained a comment: a window's size is a fact about formatting. The block is now EXTRACTED and tested, as selftest 320 already does with the runner's classifier. Then the extraction counted the block's own explanation as a second occurrence of the sentences it was counting, so comments are stripped -- selftest 080's control problem, met in a fifth file tonight. One arm was deleted rather than fixed: it read a variable defined ninety lines below it, and the block that defines the variable already asserts the same thing. Evidence: selftest exit 0, 696 checks, 0 failures; 145 pytest passed; shellcheck rc=0; docs_style PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
@linuxhikerpm's blocking finding, and they are right on a point I had got wrong. I had listed test_check_results_are_machine_readable.py in CONTEXT.md's debt inventory as "arrives with PR #923". A file that has not landed is not pre-existing debt: the harness-independence rule is already on main, and this PR would have introduced a fresh violation of it. The file sources the real test/lib.sh, executes a fixture that sources it, and extracts pgc_reconcile_records from run_all_versions.sh. Under the owner's ruling -- "each harness is independent and should only parallel test functionality" -- that is the coupling rather than the twin: it agrees with the shell by construction and can never report it wrong. DELETED RATHER THAN REWRITTEN, deliberately. The remedy the review asks for is a pytest-native equivalent, and the pytest harness has no per-assertion record stream to be native to: Expect counts assertions, and user_properties carries only the unrunnable state. So "independently implement identity, verdict, reason, sanitization, SKIP accounting and count reconciliation" is a FEATURE in the pytest layer, not a port of this file. Filed separately rather than grown onto a change that is already large. The precedent is #927, where the shell part whose subject was a python module's source text was deleted rather than repaired. What this PR keeps is the shell work, which is what the shell harness owns: strict six-field RESULT validation with the emitter's own verdict list, tab, CR and newline sanitisation, 25 named SKIP sites routed through pgc_record, the derived anti-drift sweep, and the static arm for a check_skip reading a name its file never assigns. Removed the NO_CLUSTER entry, the TESTS.md section and its TOC entry, and the CONTEXT.md debt line that named a file which will no longer arrive. Checked rather than eyeballed: 21 headings against 21 TOC entries, contiguous 1..21, every anchor equal to GitHub's derivation; membership_report []; the derived job 9 files, 161 passed. selftest 350 53/53, 400 81/81, 080 15/15, shellcheck rc=0. And the property the review turns on, measured rather than asserted: of the python files this branch touches, none now reaches into shell. test_suite_accounting.py still does, but it landed on main in #922, it is named in the CONTEXT.md inventory, and this branch's seven changed lines there add zero shell references. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
#926 and #935 landed while this waited. CHANGELOG: both sides append at the top of the same section and neither replaces anything, so the union is the resolution. TESTS.md: one region, and this side of it is EMPTY. This branch deleted its section 22 in aa07d47, and main added its own -- #935's test_writes_wrote_rows.py. Main's is the only content, so the resolution is to take it. 22 headings against 22 TOC entries, every anchor equal to GitHub's derivation, contiguous 1..22. selftest 080 goes from 15 checks to 53 because #926 landed, which is the number that branch reports and not a change of this one. On the composed tree: 350 53/53, 400 81/81, 080 53/53, shellcheck rc=0 over the whole harness, the driver-free job 10 files 174 passed, membership_report []. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
#926 and #935 landed on main and #923 dropped its coupled pytest twin, so all three reached this branch at once. NO_CLUSTER: the base removed test_check_results_are_machine_readable.py, this branch had added test_mutation_ledger.py, and the conflict spanned both. Kept the ledger entry and dropped the deleted file's. Its comment block was OUTSIDE the conflict region and survived as six orphaned lines above an unrelated entry -- removed, and the module asserted to still parse and to hold 11 entries with the deleted file absent. That is the orphaned-heading shape this repository has been bitten by before, and git will not point at it. TESTS.md: two regions. In the TOC this branch had 22 (the deleted file) and 23 (the ledger) while main had its own 22, test_writes_wrote_rows.py; in the body this side opened with six orphaned lines of the deleted section before the ledger's. Composed as main's 22 followed by this branch's 23. 23 headings against 23 TOC entries, every anchor equal to GitHub's derivation, contiguous 1..23. THE LEDGER IS REGENERATED AGAIN, and #926 is why: it took selftest 080 from 15 checks to 53, and the gate refuses a check it has never seen. From a real run of the composed tree: harness_selftest.sh: PASSED, rc=0 checks run: 773 | accounting: 773 passed + 0 failed + 0 unrunnable + 0 skipped ledger: 734 rows -> 772 | never=772, ever red=0 log triples not in the ledger: 0 All 772 rows carry five fields, none ends in a tab, and the budget's asserted census follows to 772. Gates on the composed tree: 350 53/53, 400 81/81, 410 96 checks 0 failed, 080 53/53, shellcheck rc=0, the driver-free job 11 files 183 passed, membership_report []. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
@OffgridwithJD found the fourth place. aa07d47 removed test_check_results_are_machine_readable.py from NO_CLUSTER, its TESTS.md section, its TOC entry and the CONTEXT.md debt line -- and left its six-line comment dangling before the closing bracket, still saying "this branch's own new file" about a file that is gone, and cut off mid-phrase at "should not have this". My removal walked back to the nearest `\n #` before the entry, which is the LAST line of the comment block rather than its first. Nothing executes a comment, which is why this survived every gate: the module parses, NO_CLUSTER holds the right ten entries, and no arm reads it. AND A CORRECTION TO THAT COMMIT'S CLOSING CLAIM, which was too broad. I wrote "of the python files this branch touches, none now reaches into shell". That is false: this branch touches test_suite_accounting.py, and that file does reach into shell. What I had measured was that none of the files this branch INTRODUCES does, which is the distinction @linuxhikerpm's finding turns on, and the sentence should have said so. As written it reads as "this branch is clean of shell-reaching python" and makes a tracked debt file look paid off. The counting is its own lesson. My pattern reported 5 references there, a broader one 19, and executable-only 12 -- three numbers for one file, which is the reason CONTEXT.md's own rule says to count FILES and name the mechanism. I did not follow the rule I wrote. 350 53/53, 400 81/81, the driver-free job 10 files 174 passed, membership_report []. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
@linuxhikerpm's five, each reproduced here before it was fixed. 1. CI LOGS DID NOT OUTLIVE THE BUILD DIRECTORY. The gate runs, `rm -rf "$builddir"` runs, and CI's collection step globs the build directory AFTER the loop -- so it searched a path already removed and collected nothing. That is worse than lost diagnosis: the ledger is fed by merging real logs, and a CI red is exactly the run that first records a check going red, so CI could never feed the thing it gates. The runner now copies the logs to /tmp/pgcolumnar-logs before deleting, and CI reads that. Three arms, including one on the ORDER, because copying after the delete would be silent. 2. MALFORMED LOGS WERE ABSORBED AS EVIDENCE. read_records accepted `len(f) >= 5`, so a record missing its reason, a verdict outside pgc_record's vocabulary, an empty check name, one record against `checks run: 2`, and a log with no count at all all merged at rc=0. Measured, all five. It now validates the schema and reconciles each log against its own stated count, with a control because five arms reporting rc=2 prove nothing if the tool refuses everything. 3. A MUTATION WAS ATTRIBUTED TO EVERY CASUALTY. One deliberate change can redden the target and whatever depended on it; a two-FAIL log merged with --mutation recorded it against both, which is collateral damage entered as evidence that the mutation kills that check. More than one failing check in a run is now refused, with the count, and a single failure still carries it. 4. LAST RED COULD GO BACKWARDS. It was a plain assignment: merging an older log rewrote a recent observation, an undated merge replaced a real date with `unknown`, and `--date not-a-date` was stored verbatim. Now ISO-validated and monotone, with `unknown` and `never` ranked below every date. 5. A DELETED PART WAS INVISIBLE. harness_selftest globs selftest/*.sh, and a glob cannot notice a deletion: the file goes, the loop runs once fewer, the suite reports a smaller number, every remaining check passes. Measured -- a log with all 96 records of part 410 removed and `checks run:` adjusted reconciled AND merged at rc=0. selftest/parts.manifest is now a committed list the glob is compared against, in a new part 420 rather than in the driver, because selftest 200 requires the driver to hold no checks and that rule is right. A RUNTIME "every part contributed at least one check" ARM WAS WRITTEN AND REMOVED. Parts 010 and 020 legitimately contribute none -- 020 is setup with no check calls at all, 010's only checks are on its failure path. Two false positives on a healthy tree is not a rule, and 420 says so rather than leaving the next person to rediscover it. Fixtures updated, not weakened: 410 and its pytest twin used `--date D` as a placeholder and built logs without a `checks run:` line, both of which the tightened ingestion now refuses. Those are shapes the emitter cannot produce. The base moved twice under this branch. NO_CLUSTER conflicted where #923 removed its deleted file's entry and this branch added the ledger's; kept the ledger's. Ledger regenerated from a real run of the composed tree, 795 checks and 0 failures: 794 rows, five fields each, no trailing tab, 0 log triples the ledger has never seen, budget census following to 794. selftest 350 53/53, 400 81/81, 410 96 checks to 111, 080 53/53, shellcheck rc=0, the driver-free job 11 files 186 passed, membership_report []. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…the number first commandprompt#925 adds `410-a-check-must-have-been-red.sh` and `420-a-deleted-part-must-be-visible.sh`, and commandprompt#923 under it adds `400-a-check-result-must-be-machine.sh`. Two of us picked the same numbers from the same free range, which is the residual commandprompt#554's one-file-per-subject rule leaves open: the rule stops two additions touching the same LINE, and a number is not a line. commandprompt#925 is approved and answering blockers, so it keeps the number and this part moves. The subjects never overlapped -- theirs are the red-check ledger and the part manifest -- so this is numbering and nothing else. A NOTE FOR WHOEVER MERGES SECOND. commandprompt#925 also adds `test/selftest/parts.manifest` and a part comparing it against the glob, because a glob cannot notice a deletion. If commandprompt#925 lands first, this part has to be added to that manifest or its arm reddens -- correctly, because that is the arm doing its job rather than a conflict. The reverse order needs nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…the number first commandprompt#925 adds `410-a-check-must-have-been-red.sh` and `420-a-deleted-part-must-be-visible.sh`, and commandprompt#923 under it adds `400-a-check-result-must-be-machine.sh`. Two of us picked the same numbers from the same free range, which is the residual commandprompt#554's one-file-per-subject rule leaves open: the rule stops two additions touching the same LINE, and a number is not a line. commandprompt#925 is approved and answering blockers, so it keeps the number and this part moves. The subjects never overlapped -- theirs are the red-check ledger and the part manifest -- so this is numbering and nothing else. A NOTE FOR WHOEVER MERGES SECOND. commandprompt#925 also adds `test/selftest/parts.manifest` and a part comparing it against the glob, because a glob cannot notice a deletion. If commandprompt#925 lands first, this part has to be added to that manifest or its arm reddens -- correctly, because that is the arm doing its job rather than a conflict. The reverse order needs nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
|
@linuxhikerpm — your five from Exact head 2. Malformed and unreconciled logs. Eight inputs, each refused with its own message, against a control that still passes — because eight arms reporting rc=2 prove nothing if the tool refuses everything: 3. Mutation attribution. A two-FAIL log merged with 4. 5. Deleting a shell part. The third row is why the part checks the buckets and not the totals: 42 against 42 with two parts wrong. 1. CI logs. Merge order, from @OffgridwithJDThis PR adds parts 410 and 420 and the manifest. #940 and #941 renumbered theirs to 430 and 440 to avoid the collision. If this lands first, those two parts must be added to I have not merged this or #923, and will not without the owner's word. |
…to the wrong part @jdatcmd's blocker, and it is the residual I named in my own message about the number collision: the rename moved the file and the number, and a number written into PROSE is not a line git can conflict on. `010-stand-up-a-squatter-on-a.sh:22` said "part 410 asserts the literal still equals the constant". The arm is in 430. On its own that is a dangling pointer -- but commandprompt#925 adds `410-a-check-must-have-been-red.sh`, so once that merges the reference RESOLVES, to a healthy part about the red-check ledger that asserts nothing about 66. A reader checking whether the literal is guarded would find a real file that does not guard it and stop looking. A dangling reference gets noticed; one that resolves to the wrong thing does not. TWO NON-BLOCKING NOTES, both taken, because both were numbers I retyped rather than measured. "A flat grep sees 13 either way" is true of main and false of the branch it ships in. Measured on this head it sees 24, because part 430's own fixtures add eleven `exit 0` lines. The real numbers argue the point better than the round one did, so the comment now carries both and says why the flat count moves for a reason unrelated to the defect: heredoc-aware sweep 2 before the fix, 0 after flat grep -c 'exit 0' 13 before, 24 after And the sweep closed ONE SPELLING of the mode. `exit 0` ends the driver with success; so does a BARE `exit`, which exits with the last command's status and is very often 0. That one is catchable, so the sweep now catches it, with an arm planting it and a control asserting a deliberate `exit 66` is not an offence. `exit $?` and `exit "$rc"` cannot be decided statically -- flagging them would refuse a part that legitimately exits non-zero -- so the comment names them as what the sweep does not see. Measured on this head: bare `exit` 0 sites, `exit $?` 0, `exit "$var"` 0, so nothing is open today. Gate: 606 checks, 606 passed + 0 failed + 0 unrunnable, PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…eadable branch Part 010 conflicted, and resolving it found a latent defect of MINE that main's side fixes. Taking main's side wholesale, not as a compromise. THIS BRANCH KEPT `exit 0` IN A SOURCED PART, which is exactly the defect #934 names and #940 closed. My change to part 010 swapped the bare `echo "SKIP ..."` for `check_skip`, to route the outcome through pgc_record as @linuxhikerpm asked, and left the `exit 0` below it untouched. AND THE `check_skip` COULD NEVER HAVE RECORDED ANYTHING. `lib.sh` is sourced by part 020; part 010 runs before it, which is the same fact #940's own comment turns on when it spells 66 as a literal rather than `$PGC_EXIT_SKIPPED`. So `check_skip` in part 010 is not a call to a helper, it is a command that does not exist yet. Measured rather than argued: type check_skip, at the point part 010 runs NOT DEFINED calling it under the driver's `set -uo pipefail` bash: check_skip: command not found, rc=127, execution CONTINUES Under `-u -o pipefail` without `-e` that is not fatal, so the bail path would have printed a bash error, recorded nothing, and then exited the driver with status 0 -- the false green, with a diagnostic line in front of it. It has never fired because the squatter-port path does not trigger in a normal run, which is why no gate has ever executed those two lines. main's version is correct on all three axes: `exit 66` rather than `exit 0`, the `SKIPPED (ran no checks)` marker the runners pair with 66, and no call to a function that has not been defined yet. PART 010 IS THEREFORE AN EXCEPTION TO THE RULE THIS PR ARGUES, and it should be named as one rather than quietly left out. Every per-check outcome routes through `pgc_record` except in the one part that runs before `pgc_record` exists. There the right shape is a part-level bail, which is what #940 built. Checked rather than eyeballed, on the merged tree: parts sorting before 020 010 only part 010: exit 0 sites / exit 66 / markers 0 / 2 / 2 part 010: check_skip calls 0 part 430's arms: PGC_EXIT_SKIPPED=66 in lib.sh 1 exit-0 and bare-exit sweep over all parts 0 driver handed a bad pg_config rc=2, no summary accounting assertions still on the 4-field form none driver-free pytest job 10 files, 183 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
One conflict, in `run_all_versions.sh`, and BOTH SIDES WERE RIGHT ABOUT
DIFFERENT THINGS. Taking either alone regresses the other:
mine the accounting pattern gained `\+ [0-9]+ skipped`, the 5-field form
this branch introduces -- but still compared with `!= 0`
main compares with `-ne 0`, #941's fix for the empty-count inversion
-- but on the 4-field accounting pattern
Resolved to the union: the 5-field pattern with the numeric comparison. Measured
which regression each half would have been, rather than asserting that both
matter:
taking my side alone #941's part 440 sweep goes non-zero
taking main's side alone the 5-field accounting pattern is gone
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
#941 landed the rule and its sweep while this branch was open, and the sweep reads every part -- so these four went from "the old idiom" to an offence the selftest refuses, without anyone editing them. They are MINE, not pre-existing. On main, `320-a-check-that-could-not-run.sh` holds zero string-compared `grep -c` sites; this branch added four: 254 grep -c 'pgc_summary' "$_cnt_f" 283 grep -c 'pgc_summary' "$_f" 295 grep -c 'pgc_summary' "$_f" 296 grep -c 'PGC_CHECKS=\$((PGC_CHECKS' "$_f" So this branch reintroduced #929's defect four times while #941 was closing it: `grep -c` prints NOTHING on a pattern that does not compile, and `[ "" != 0 ]` is TRUE, so each of these answers "present" for a question grep never managed to ask -- and three of the four are premise arms phrased to want present, which is the direction that turns green when the instrument breaks. All four pass exactly ONE input to grep, checked rather than assumed, which is #941's condition for the conversion being behaviour-preserving: `grep -c` over several files prints `file:count` lines, which a numeric comparison would reject where the string form tolerated it. No site here does that. #941's sweep on the composed tree, before 4 after 0 and still a live zero: one planted back 1 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
…ew rules (#918) Merging the updated #923 base, which now carries #939, #940 and #941, puts this branch under two rules that did not exist when it was written. Neither is a conflict git could have shown me: both are arms that read the WHOLE tree, so a file this branch never touched changed what this branch has to satisfy. THE MANIFEST, which is this PR's own arm firing correctly. Part 420 compares `parts.manifest` against the glob in both directions. #940 and #941 added parts 430 and 440 to main, so the manifest was two short and the arm would have gone red -- which is the guard working rather than a merge problem. @OffgridwithJD called this exact ordering on both of their PRs before either landed. on disk 44, listed 42, only-on-disk: 430-..., 440-... after, on disk 44, listed 44, both directions empty still live: drop 430 from the comparison and it is named again Added in sorted position rather than appended, because the file is sorted and a manifest that stops being sorted is a diff nobody can read. AND ONE MORE STRING-COMPARED `grep -c`, MINE, at `run_all_versions.sh:1382`: [ "$(grep -c '^RESULT\t' "$builddir/${s}.log" || true)" != 0 ] Zero such sites on main and zero on the #923 base, so this branch added it -- #929's defect, reintroduced while #941 was closing it. `grep -c` prints nothing on a pattern that does not compile and `[ "" != 0 ]` is TRUE, so this answers "the log has RESULT records" for a question grep never asked. One input to grep, checked, which is #941's condition for the conversion being behaviour-preserving. #941's sweep on the merged tree, before 1 after 0 still live: one planted back 1 Checked on the merged tree, not on this branch alone: #940's exit-0 and bare-exit sweep 0 the driver handed a bad pg_config rc=2, no summary ledger control / BOGUS verdict / one rc=0 / rc=2 / rc=2 --mutation against two failures driver-free pytest job 195 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
linuxhikerpm
left a comment
There was a problem hiding this comment.
Blocking at exact head 54cb94c09a59. The five integrity findings from 71a7c9bc look addressed in 7dffd28c (log keep-then-delete, schema/reconcile, one-mutation-one-FAIL, ISO/monotone last-red, parts.manifest). CI on this SHA is not green, and the failure is the gate this PR added.
PG17 and PG18 suites both fail the ledger gate:
32 check(s) the ledger has never seen
harness_selftest 430-the-self-test-must-not-report …
harness_selftest 440-a-count-grep-never-produced …
PG17 has a check the ledger has never seen, which is not a pass
54cb94c0 updated parts.manifest for 430/440 after merging #923/#941, but check_ledger.tsv still has 794 rows and does not contain those parts' checks. Merging main into a ledger branch has to regenerate the ledger from a real run, or the gate refuses the merge that was supposed to keep it current. That is the allowlist working; the tracked file is stale relative to the composed tree.
Also, the same log printed:
no budget at refs/remotes/origin/feat/917-machine-readable-results (1 commit behind HEAD):
this change introduces it, so there is no prior ceiling it could have raised
--against auto is documented to use GITHUB_BASE_REF (this PR's GitHub base is main). Please confirm why CI resolved the prior to the #917 branch. If auto follows the stacked topic branch rather than the PR target, the ceiling comparison is not the one the review of 71a7c9bc asked for.
Regenerate the ledger on the composed tree, re-run, and I will re-review that SHA.
…#918) CI refused this branch, and the refusal was this PR's own gate working. Every suite passed; the gate stopped the run because parts 430 and 440 -- #940 and #941, which landed on main after this ledger was generated -- contribute checks the ledger has never seen: ledger census: rows=794 | never observed red=794, ever red=0, new this run=32 PG17 has a check the ledger has never seen, which is not a pass I HALF-LEARNED THE LESSON @OffgridwithJD ALREADY GAVE ME. They called the parts.manifest ordering before either of their PRs landed, and I updated the manifest for 430 and 440 -- and did not then ask what ELSE in this PR is a tree-wide artifact that a new part invalidates. There are two, and I fixed one. The manifest lists parts; the ledger lists CHECKS, so it moves whenever any part gains or loses one, which is far more often. REGENERATED FROM A REAL RUN, not from the CI log. The failing job prints all 32 as `not in the ledger: suite<TAB>part<TAB>name`, so they could have been parsed out -- but a ledger whose rows came from someone reading a log is the artifact this ledger exists to replace, and the merge path validates a log against its own `checks run:` count, which scraped text would not have. bash test/harness_selftest.sh /usr/local/pg17/bin/pg_config rc=0, checks run: 827, accounting: 827 passed + 0 failed + 0 unrunnable + 0 skipped = 827 of those, 32 are from parts 430 and 440 -- the number CI named MY FIRST RUN OF THAT WAS UNUSABLE AND I NEARLY MERGED IT. I copied the tree without `.git`, and 15 checks failed on "premise: the source tree is a git checkout: got [no]". Merging that log would have written 15 environment-induced reds into a ledger whose entire subject is which checks have ever been red -- poisoning the record with the one kind of entry it must never contain. Re-run from a real checkout: 0 failures. The merge is additive and nothing existing moved: ledger 794 -> 826 rows, delta 32 added: 18 from 430-the-self-test-must-not-report, 14 from 440-a-count-grep-never-produced rows removed or altered: 0 And the comparison that failed CI, run both ways so it can fail: checks in the run absent from the OLD ledger 32 (18/14, as CI said) checks in the run absent from the NEW ledger 0 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
…l of them (#918) Regenerating the ledger moved a number the budget asserts, so the pytest guard went red: `the committed census matches the committed ledger: got 794 want 826`. checks_never_observed_red 794 -> 826 THIS IS THE THIRD TREE-WIDE ARTIFACT IN A ROW, and the first two I found by being told. @OffgridwithJD called the parts.manifest before their PRs landed; CI caught the ledger; this one I found by running the guard. So this time I enumerated instead of fixing what broke and pushing: every occurrence of 794 in the tree ONE, check_ledger_budget.txt:37 part 410, the shell twin DERIVES it, `sed -n 's/^checks_never...'` compared against the ledger -- no copy So there is no fourth. The shell twin cannot drift because it does not hold the number, which is the design the budget file's own comment argues for. THE CEILING IS NOT TOUCHED, and that distinction is the whole point of the file. `suites_not_covered` is monotone and may only FALL; the gate refuses an increase. `checks_never_observed_red` is a CENSUS -- a measurement that must be true, not a bound -- because every new check enters the ledger as `never`, so bounding it would deadlock. Both re-derived rather than assumed: census committed 826 vs ledger 826 agree ceiling committed 250 vs computed 250 agree, unmoved Adding checks to `harness_selftest`, which already has rows, cannot move the ceiling -- exactly as the file predicts. Proved by removal, both directions, against a green control: revert the census to 794 test_the_committed_ledger_and_budget_agree FAILS raise the ceiling to 9999 the same arm FAILS control 12 passed Both harnesses, on the merged tree: pytest driver-free job 11 files, 195 passed harness_selftest PG17 rc=0, 827 passed + 0 failed + 0 unrunnable + 0 skipped = 827, part 410's census arm PASS Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
…th file arrived Rebased onto main (d05e3c3). Deleting `test/selftest/370` turned out to touch three artifacts beyond the part itself, and the rebase surfaced a new item of debt that the inventory arm was built to catch. Artifacts the deletion reaches: - `test/selftest/parts.manifest` still named 370. commandprompt#925's arm "every name in the manifest is a part on disk, so a deletion reddens" caught it, which is the arm doing precisely its job on the first deletion after it landed. - `test/check_ledger.tsv` held 12 rows for 370, all `never`. Removed. - `test/check_ledger_budget.txt`'s census `checks_never_observed_red` goes 826 -> 814. DERIVED from the ledger after the removal rather than typed, because that file says of itself: "it is not a ceiling; it is a measurement that must be true". `suites_not_covered` stays at 250 — deleting a part removes no suite. The 12 ledger rows are also the evidence for the claim made when 370 was deleted. They include "a neutered present arm is caught" and "premise: plan_marker's body was actually cut out of the file": 370 cut the Python body out and re-grepped its own detector. Every arm was a text pin, which is what commandprompt#927's precedent is about. A fourth coupled file arrived with commandprompt#925: `test_mutation_ledger.py` runs `run_all_versions.sh --list-suites`. The set-equality assertion in `test_harness_deps.py` reddened on the rebase naming a file the declaration did not have — the direction the inventory was made a mechanism for, working on something nobody wrote it for. It is now declared, with its mechanism, and CONTEXT.md records both the file and the fact that the arm is what found it. Same mechanism as `test_suite_accounting.py`, so the two are one item of debt twice and should move together. Verified on pg18a: selftest 815 checks, 0 FAIL; driver-free subset 196 passed; the full corpus green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
…mpt#946) `merge` already refused a log that does not RECONCILE, and reconciliation is not the property that matters. Both logs that poisoned this ledger on the day it landed reconciled: - mine, on commandprompt#925: 827 records against `checks run: 827`, fifteen checks red because I had copied the tree without `.git`. Flawless self-consistency, fifteen environment reds, and I caught it by noticing the failures rather than by the tool refusing them. - @OffgridwithJD's, on commandprompt#944: one FAIL from an unfinished change, recording a red for part 350's doc-coverage check that the check never earned. Two independent routes on day one, from the two people who knew the tool best. A third costs no imagination: a run against a stale `.so`. THE RATE IS THE ARGUMENT, not either instance. AN ENVIRONMENT RED AND A REAL REGRESSION ARE IDENTICAL IN THE LOG. Nothing in a RESULT record says which, so the tool cannot infer it and now makes the caller assert it -- the same move `check_ledger_budget.txt` already makes when it names a census apart from a ceiling. `merge` refuses a log carrying any FAIL unless the caller says which kind of red it is: `--mutation NAME` (exists, a deliberate break) or `--reds-are-real` (new, a genuine observation of the code under test). Refused BEFORE any row is built, so a declined merge is never half-applied. NOT "refuse FAILs unless --mutation", which @OffgridwithJD proposed and we rejected together. A genuine CI red is the most valuable row this ledger can hold and has no mutation to name, so that rule would refuse precisely the entry the ledger exists for -- the deadlock the budget file already argues against for `checks_never_observed_red`. A DEFAULT REFUSAL IS AFFORDABLE BECAUSE NOTHING AUTOMATED CALLS `merge`. `grep -rn 'pgc_ledger.py merge' .github/ test/` finds one hit and it is the gate's own advice string. `gate` is the automated path. That was the cost I expected to have to weigh and it does not exist. Both harnesses, independently implemented. The shell twin builds its own fixtures and names no pytest file; the pytest twin drives the tool directly. EIGHT EXISTING CALL SITES UPDATED, AND I CHECKED EACH RATHER THAN CHASING GREEN. Four arms broke, all of them using a red log as a VEHICLE for a different subject -- duplicate detection, rename grouping, date monotonicity, mutation attribution. None asserts that a red merges without a reason, so passing the new flag keeps each testing what it tests. One needed more than that: the control in `test_a_mutation_names_one_check_not_every_casualty` was named "the same log merges without --mutation", which stops being what it demonstrates. It now says the refusal above is --mutation-across-two-checks and not the log. Proved by removal, and by a control that must not move: guard neutered (`if False`) the new arm FAILS, alone control an all-PASS log merges with no flag, rc=0 driver-free pytest 196 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
Seven new checks in part 410, so two tree-wide artifacts move with them. I enumerated what a new check obliges rather than fixing whatever broke first, which is the lesson commandprompt#925 cost three rounds to learn: TESTS.md entry done in the first commit NO_CLUSTER membership already true, test_mutation_ledger.py is driver-free shell twin done in the second commit parts.manifest part 410 already listed, unchanged check_ledger.tsv +7 rows, this commit check_ledger_budget.txt census 826 -> 833, this commit REGENERATED FROM A GREEN RUN, and this change is its own demonstration: the log is 833 records with zero FAILs, so the guard added here does not refuse it. A red run would now be refused by the very thing being added, which is what commandprompt#925's `.git`-less regeneration needed and did not have. harness_selftest, PG17 rc=0, 833 passed + 0 failed + 0 unrunnable + 0 skipped = 833 ledger 826 -> 833 rows, all 7 in 410-a-check-must-have-been-red rows removed or altered 0 census 826 -> 833, DERIVED from the ledger, not typed ceiling 250, untouched -- adding checks to a covered suite cannot move it, as the budget file predicts driver-free pytest 196 passed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw
Closes #918 — phase 4 of #858, the last of the three. Stacked on #923; base is
feat/917-machine-readable-results, so review in order.The honest version, and why that is the finding
Nothing recorded whether a check had ever been red. That is the gap that let 39 checks across 35 suites ship unable to fail, three of them inside the suite whose whole purpose is to stop exactly that.
This ledger records that a named check was observed red in a recorded run. It does not claim the check is proven able to fail. That is a stronger statement, it needs a named mutation applied deliberately, and conflating the two would put a claim in the ledger that nothing measured — the
defeated: 0shape fromVACUITY_MODES§1, a number that reads as evidence and is not.So every entry currently reads
never. #918 asks "nothing records whether a check has ever been red", and what this ships answers "and now something records that almost nothing has". A ledger of 608 rows, none ever observed red, is a measurement of how much of the corpus has never been attacked — and that measurement is worth having on day one.What fills it
Not only deliberate mutation runs. Every real CI red fills it, every flake, every bisect — and those arrive whether anyone remembers to run something or not. A mutation run is the deliberate accelerator, not the only source. Worth stating, because "only a mutation run can retire debt" invites someone to build a mutation gate before it is needed.
The format
The mutation column exists from v1 with nothing filling it automatically, because adding a column later means rewriting every entry. If an entry can record which mutation reddened a check, the catalogue a mutation gate would need builds itself out of work people already do by hand.
Two losses that are reported rather than prevented
A rename. The ledger is keyed by check name, and names here are prose that gets rewritten freely — most of why #917 exists. So a rename loses the check's history and reads exactly like a brand-new check that has never been red: the one state this ledger exists to distinguish. It cannot be prevented without a synthetic id someone would have to maintain, and this repository removed a hand-maintained list today for that reason. So a name that appeared while another disappeared is named. Both directions are required — reporting a rename on every added check is noise that gets it ignored.
A duplicated name. Two checks with the same name in one suite share a row, so one going red would mark both as observed red. Reported for the same reason. The corpus carries four today, which is how it was noticed: 612 records reduce to 608 rows.
The debt is two tracked files
Per #858's own constraint.
test/check_ledger.tsvandtest/check_ledger_budget.txtare in the tree, so a change to either is a diff a reviewer sees.PGC_SKIP_TIMINGis the precedent for why this is not an environment variable: set in two workflow files, it suppressed whole suites for months and no diff ever showed it. Both numbers may only go down.The second is the one that is easy to forget, and it is a real limit. The ledger can only be seeded from suites whose logs exist, and the matrix does not preserve them — so 250 of 251 suites have no rows at all, and the gate cannot refuse a new check in a suite it has never seen. Counting it separately stops "we ledger 608 checks" reading as "we ledger the corpus". It burns down as suites are seeded.
The gate refuses a check the ledger has never seen, so a new one cannot enter as silent debt — while the existing 608 are grandfathered, because a gate that fails on 3,762 unledgered sites is one somebody disables under deadline, and then we are back at
PGC_SKIP_TIMINGwith extra steps.Evidence
What I did not build, deliberately
A harness-driven mutation gate. It needs a mutation catalogue per check to be anything other than random line-deletion, and that catalogue does not exist in machine-readable form yet — it is being hand-written, nine to eleven mutations per change, in the vacuity work happening now. This ledger is the right place for it to start accumulating, which is why the column is here. Building the gate first would be building the machine before the input.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK